-
Notifications
You must be signed in to change notification settings - Fork 401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: new module: avm/res/network/network-security-perimeter
#4278
base: main
Are you sure you want to change the base?
Conversation
Could somebody take a look at this PR pls? |
I'll jump in here @peterbud. |
@@ -133,6 +133,7 @@ | |||
/avm/res/network/network-interface/ @Azure/avm-res-network-networkinterface-module-owners-bicep @Azure/avm-module-reviewers-bicep | |||
/avm/res/network/network-manager/ @Azure/avm-res-network-networkmanager-module-owners-bicep @Azure/avm-module-reviewers-bicep | |||
/avm/res/network/network-security-group/ @Azure/avm-res-network-networksecuritygroup-module-owners-bicep @Azure/avm-module-reviewers-bicep | |||
/avm/res/network/network-security-perimeter/ @Azure/avm-res-network-networksecurityperimeter-module-owners-bicep @Azure/avm-module-reviewers-bicep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently shows an error either because
- The required teams (as per the contribution guide) don't exist - OR
- The teams do exist, but are not added as children to the correct owner/contributor parent teams - OR
- The teams do exist, and the request to add them to the correct parent teams is not yet approved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just created the teams now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @peterbud,
much appreciated, but that alone is not enough. As written in my initial comment, and as documented in the contribution guide here, the teams must also be
- Added to the hierachy of the correct contributor & owner teams
- Set 'adding' must be approved by the POs
This is required to ensure you, as the module owner, are able to later approve PRs for your own modules that were raised by a 2nd part.
Also on a sidenote: If possible, please only resolve comments if they're 'code suggestions' (the ones with the commit button) as all others should be re-reviewed by the person that created them. Otherwise a re-review like in this case is a bit tedious as I can't outright see what I originally commented on and if it was fixed. A comment 'addressed' below the review comment would be ideal, thanks :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the last one - I just checked the code an can see that actually none of the code suggestions below were addressed. If you applied them locally, you may have missed to push your changes. I'll go ahead and unresolve them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have started working on this (therefore you see some resolved as I tracked them with the VSCode extension), but had to stop, and I'll push it later, when all feedback will be handled properly. Sorry for the confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, all changes has been pushed, and resolved. pls have a look.
The contributor and owner teams are in place, they need to be approved by the parent owners.
Description
Adding new module
avm/res/network/network-security-perimeter
See Azure/Azure-Verified-Modules#1709
Pipeline Reference
Type of Change
version.json
:version.json
.version.json
.Checklist
Set-AVMModule
locally to generate the supporting module files.Pls. note that network security perimeter should be specifically enabled in the subscription:
https://learn.microsoft.com/en-us/azure/private-link/create-network-security-perimeter-portal#prerequisites